-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to Apache Lucene 9.11.0-snapshot-4be6531 #13850
Conversation
@reta needs your review here. |
❌ Gradle check result for 397ac41: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Seems like it needs more work, Apache Lucene updates are rarely seamless ... |
yeah.. fixing the issues now. |
@reta the whole idea of updating snapshot was to make this change: server/src/main/java/org/opensearch/plugins/PluginsService.java into Opensearch. What would be the recommendation here to add this SPI reloading change? Should I create a new PR or use the same PR? I want to ensure that whenever 9.11 is released the reloading of KNNVectorsFormat SPI also comes to 2.x branch. |
@navneet1v I think it depends on the change: if it is more or less trivial -we could do it in this pull request, if it needs some non-trivial code changes, having the separate pull request (and backport to 2.x once 9.11 is released) would be preferred. |
@reta The reason why I was reluctant to raise another PR is, whenever backport of Lucene 9.11 version will happen in core I don't want that change to be missed as it will break k-NN plugin. Hence thinking to use this PR only. Any suggestions? |
❕ Gradle check result for d18fb26: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13850 +/- ##
============================================
+ Coverage 71.42% 71.57% +0.15%
- Complexity 59978 61289 +1311
============================================
Files 4985 5063 +78
Lines 282275 288030 +5755
Branches 40946 41710 +764
============================================
+ Hits 201603 206167 +4564
- Misses 63999 64875 +876
- Partials 16673 16988 +315 ☔ View full report in Codecov by Sentry. |
Please raise the separate pull request, we will backport it, thank you. |
server/src/main/java/org/opensearch/search/fetch/subphase/highlight/FastVectorHighlighter.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/lucene/search/uhighlight/CustomFieldHighlighter.java
Show resolved
Hide resolved
Signed-off-by: Navneet Verma <[email protected]>
❌ Gradle check result for fe4edef: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for fe4edef: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
@reta thanks for merging the code and running the gradle checks again. |
…3850) Signed-off-by: Navneet Verma <[email protected]>
…3850) Signed-off-by: Navneet Verma <[email protected]>
Description
Update to Apache Lucene 9.11.0-snapshot-4be6531 for main branch only, as 9.11 version of Lucene is not released.
There are some changes that made some changes in highlighter classes. I have fixed those. One of the major was adding of a Passage Comparator. As of now I have used the default comparator
Comparator.comparingInt(Passage::getStartOffset)
which was getting used earlier.Ref: apache/lucene#13276
Related Issues
Resolves #13848
Check List
New functionality includes testing.New functionality has been documented.New functionality has javadoc addedAPI changes companion pull request created.Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)Commit changes are listed out in CHANGELOG.md file (See: Changelog)Public documentation issue/PR createdBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.